Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Problem child ingest pipeline bug #11367

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Problem child ingest pipeline bug #11367

wants to merge 8 commits into from
+7 −2

Conversation

r0ot
Copy link

@r0ot r0ot commented Oct 8, 2024

Proposed commit message

The painless script if block serving as the entry point into the problem child inference pipeline has a bug in its logic checking for the appropriate host operating system. It separates its checks for various sub-fields of host.os and it's actual accessing of those fields from the ctx object, potentially leading to errors trying to access keys that don't exist.

For example, if a document contains host.os.family but doesn't contain host.os.type, this painless script will raise an error instead of simply failing.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@r0ot r0ot requested review from a team as code owners October 8, 2024 19:21
@andrewkroh andrewkroh added bugfix Pull request that fixes a bug issue Integration:problemchild Living off the Land Attack Detection Team:Security-Applied ML Elastic Security Protections Machine Learning (ML) Team [elastic/sec-applied-ml] labels Oct 8, 2024
@sodhikirti07
Copy link
Contributor

@r0ot Could you provide an example document that is causing the pipeline errors?

@r0ot
Copy link
Author

r0ot commented Oct 8, 2024

@r0ot Could you provide an example document that is causing the pipeline errors?

I don't have a real-world document I could provide but the example I gave in my description should be sufficient. If the user's datasource isn't elastic endpoint and only provided host.os.family or host.os.platform without host.os.type it would trigger this bug.

@susan-shu-c
Copy link
Member

susan-shu-c commented Oct 9, 2024 •
edited
Loading

Hi, @r0ot we are testing this, can you also share what Elastic stack version you are using, as well as the ProblemChild/Living off the Land Detection package version you are on?

@r0ot
Copy link
Author

r0ot commented Oct 9, 2024

Upon testing, this doesn't raise an error like I thought, it rather just returns false. The notation used here could be cleaned up with ?. accesses to make the logic more concise and avoid confusion in the future. I've updated this PR with the updated notation.

So not a bug but an improvement.

@r0ot r0ot requested a review from andrewkroh October 10, 2024 17:32
@susan-shu-c
Copy link
Member

Hi, thanks again for the PR; for now we're considering rolling this up into the next release we make for this package. Will keep you posted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh Awaiting requested review from andrewkroh

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
bugfix Pull request that fixes a bug issue Integration:problemchild Living off the Land Attack Detection Team:Security-Applied ML Elastic Security Protections Machine Learning (ML) Team [elastic/sec-applied-ml]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@r0ot @sodhikirti07 @susan-shu-c @andrewkroh